Repository navigation
Adopt shared controls and semantic styles across Buzz - #88
Conversation
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
…88-refresh Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed bd5cf3f1626ef57a0bde026da9923677c1b7fbed against stacked base 338bbb5be64cb9bcd312ca153cc49ce402f0332f (#87), not main.
[P2] Synchronize shortcut journeys with asynchronous dialog dismissal
tests/browser/shortcuts.spec.mjs:44–46,175–183, following the PageSearch migration to shared Dialog.
Both journeys press Escape and immediately issue the Settings chord. Unlike the previous native dialog, Base UI closes through an asynchronous popup/focus lifecycle. The key can therefore arrive while the closing popup still has aria-modal="true"; the shortcut dispatcher correctly considers that state modal and declines navigation. The subsequent Settings/Plugins assertion then times out. This is a test-ordering defect introduced by the migration, not evidence that Settings is broken after dismissal completes.
This already fails the hosted WebKit shard. At the exact head, the unchanged shortcut file passed 7/8 locally, failing the Settings case in WebKit. An instrumented copy independently failed the plugin case and captured the blocked chord with data-closed, aria-modal=true, and hidden=false. A copy adding only a popup-removal barrier passed all 8 cases across Chromium/WebKit, with no production changes.
Required fix: establish semantic dismissal completion in both journeys (popup detached and return focus settled) before changing focus or firing the next shortcut. Preserve assertions that shortcuts are blocked while modal and work afterward. Do not add sleeps/retries or weaken the production modal guard to accommodate the race. Run the complete shortcut file in both engines and resolve the affected hosted gate.
Scope and validation
All three complementary review lanes returned and were reconciled. No other actionable introduced defect was established in the reviewed adoption paths. The complete existing GIF file passed 6/6 locally across both engines, including Emoji/GIF query handoff, keyboard selection, clearing, retry and insertion. Existing hosted CI provides broad coverage; I did not rerun the full app/native suites. Native desktop visual acceptance remains unverified. The inherited #87 Tooltip finding is separate and is not counted again here.
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
…tings Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
Signed-off-by: Arjun Mahanti <arjun@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed 622990a1dce41613f7dcad9f565883596bd69b15 against stacked base 1b5a4fcf597b9a44fd067e1ed1a3c3cde83aada8 (#87), not main.
No new blocking findings
The prior P2 shortcut-dismissal race is fixed. Both affected journeys in tests/browser/shortcuts.spec.mjs:44–50,185–193 now wait for the “Find a page” dialog to be absent, including hidden/closing instances, and for focus to return to its trigger before issuing the Settings chord. Modal-blocking assertions remain; the production shortcut guard is unchanged. No sleeps, forced clicks, or retry-based suppression were added to those fixes.
The re-review also covered changed adoption/integration seams: shared Button/Tooltip composition, connected sidebar row ownership and callback stability, Dock permission actions, confirmation pending/dismissal, media/picker control migration, semantic-role owners and adoption enforcement. Typeahead and thread-opening test changes retain their behavioral assertions and add state-based readiness checks. All three bounded source-review passes returned and were integrated, with coordinator verification of the remaining guard and production diffs.
Validation and limits
- Read-only source review on the pinned Blox bare object store. No checkout, repository installation, build, test execution, or CI rerun during this review.
- Independently queried existing checks for this exact head: 12/12 successful, including Chromium and WebKit browser shards (run 35667990538). Earlier local browser counts belong to their earlier snapshots, not this review.
- This is not native desktop/Dock acceptance or a complete rendered accessibility/visual audit. Inherited #73 settings-layout and #86 legacy-radius findings remain separate parent-stack issues; this clear review does not resolve or certify them.
No approval is being granted by this comment.
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed a5669f2406f9f9820c1bf23d153b38ad3e8ee511 against stacked base 3b276793123aea43c23767d8c93f4e262b2a5872 (#87), not main. Compared both with the previously reviewed head/base 622990a1dce41613f7dcad9f565883596bd69b15 / 1b5a4fcf597b9a44fd067e1ed1a3c3cde83aada8.
No new blocking findings
Both independent source-review lanes returned and were integrated. The four conflict resolutions retain the inherited mention provider, local-agent filtering, admission explanations and enrollment/cancellation wiring alongside shared-control adoption. The agent-page extraction retains its new owners, with the count-badge token edit relocated to AgentLibrary.tsx:74. The accepted shortcut dismissal/focus barriers, sidebar callback/memo ownership and thread-trigger readiness fixes remain intact.
The final commit is exactly seven substitutions in four files. Six preserve the existing error/warning palette values through semantic roles. AgentControls.css:20 intentionally changes the 2px focus outline from purple to border-focus (black in light mode, white in dark mode); it is not a color-preserving no-op. Those roles resolve in both themes, and the host imports the shared forms/overlay styles without introducing another reset or appearance owner. Including the relocated AgentLibrary edit, the refresh carries eight token substitutions across five inherited agent files.
Validation and limits
- Read-only source review on the pinned Blox bare store. No checkout, installation, build, tests, repository guard execution or CI reruns. Unchanged accepted paths were checked by content identity rather than exhaustively re-reviewed.
- Independently queried existing checks at this exact head: 11 successful, one skipped (Windows native validation), including successful Chromium/WebKit journey shards and the required aggregate (run 35683088011).
- This does not establish native desktop/Dock or rendered visual/accessibility acceptance, and does not clear inherited #73 settings-layout or #86 legacy-radius findings or formal review gates.
No approval is being granted by this comment.
Post-#87 squash integration repair (2026-09-22)
Current head
6915669958ba5f0d6f5be1ec19bb9a533b2d3bf5now targets main09565c31ef3e125ac877025911028eae405b8172. #87 was squash-merged, so GitHub retargeted this PR and lost the shared ancestry. Main's tree is identical to the already-incorporated #87 parent3b276793123aea43c23767d8c93f4e262b2a5872(treeea4480c473afd6bb6179da6502be13932b004f44).This repair is a normal merge, not a history rewrite. It resolves five conflicts while preserving the prior #88 head
a5669f2406f9f9820c1bf23d153b38ad3e8ee511byte-for-byte (tree401c2d153688cfc10b46baf0e8807ebe36959ba9). The new diff against main equals the previous diff against #87 exactly. No production, test, dependency or guard changes; all previous fixes remain.Independent bounded source review confirmed both tree identities. Normal hooks at the new head passed app/viewer TypeScript, 2,017 unit tests in 193 files (17.90s), and all design/adoption guards. All 62 commits in the current PR range have sign-off trailers. No browser scenarios were added or removed; no browser/native rerun was added for this content-identical merge.
The prior head's hosted CI passed (run 35683088011); Windows native validation was skipped. That is evidence on the identical prior tree, not a fresh run on this merge commit. Fresh hosted CI and the existing formal requested-changes review remain separate gates. This repair grants no approval and does not establish native or comprehensive rendered visual/accessibility acceptance.
Earlier main refresh for morning review (2026-09-22)
Earlier head
a5669f2406f9f9820c1bf23d153b38ad3e8ee511retained the stack on refreshed #873b276793123aea43c23767d8c93f4e262b2a5872; both contain main0beb523443557c189e5fcf16a3ac9e8437666690. Merge890af02resolves the four actual conflicts without rewriting history. Main's mention provider/wake notice, local-agent filtering, enrollment/cancellation and managed-agent page remain; #88's shared controls, admission explanations, sidebar memoization, shortcut-dismissal and thread-input-readiness fixes remain. The single AgentLibrary color edit follows main's extraction into its new owner.The normal push hook exposed seven raw palette references in main's new agent editor. Follow-up
a5669f2maps them to existing semantic roles, seven substitutions across four files, with no dependency/behavior/guard/test changes. Error and warning colors retain their exact token values; agent-field focus changes from purple to the documented neutralborder-focusrole.Validation:
84391eb5d42e5a6f439769ff168757cd3c2dac65passed 69 design tests, app/viewer production builds, 110 host cases across nine complete browser files (1.4m), and 38 viewer cases (21.4s), in Chromium and WebKit. Host files: mentions, typeahead, shortcuts, agent-control, plugin-import, design-system, appearance, agents, navigation-thread-history. The temporary viewer config changed only its occupied preview port and was removed.401c2d153688cfc10b46baf0e8807ebe36959ba9passed 32 cases in the complete agent-control, agent-models and agent-editor-grid files in both engines (37.2s). Normal final push hooks passed app/viewer TypeScript, all 2,017 unit tests in 193 files (17.28s), and every design/adoption guard. Committed repair exactly matches the tested diff. Every one of the 20 commits against refreshed Compose shared dialogs, tooltips and navigation #87 has a DCO trailer.Both actual branch heads are pushed. This is an integration refresh, not approval, merge, native acceptance or release certification. Fresh hosted CI and existing formal requested-change review gates remain separate; historical evidence below belongs to its stated snapshots.
What changes
Buzz's shared components and semantic tokens now own common presentation across Settings, channels, Sessions, message actions, activity, pickers, workflows and media controls. Changing the design system updates those screens together. Base UI remains responsible for the relevant keyboard, focus and choice behavior; fonts, assets and fixtures remain suitable for the public repository.
This is 4 of 4: #73 → #85 → #86 → #87 → #88. Originally stacked on #87; now review against main after #87 merged. Current main's Sessions, drafts, unread state, agent admission, Phosphor icons, agent squircles, presence, Dock badge and development notification pause are preserved.
Adoption and review fixes
surface-inversewithtext-inverse. The role is limited to existing media stages, with contrast coverage.The thread-input fix addresses a captured Linux WebKit failure: geometry had settled while Virtua still applied
pointer-events: none. Playwright's alternate scroll alignment restarted the pointer lock, so the thread never opened. The scoped computed-style readiness check adds no delay, retry, forced click or product change. Independent review confirmed this lifecycle boundary.Ownership that stays with features
The rich message editor owns caret, IME and completion behavior. Emoji Mart keeps its shadow-root adapter. Media renderers own media pixels, tile geometry, native zoom range, modal focus, dragging, playback and timecodes. Features retain layout, scrolling, data loading and subscriptions. Plugin compatibility aliases forward to shared semantic roles. This is shared presentation ownership, not a replacement for those feature contracts.
Validation
Historical head:
622990a1dce41613f7dcad9f565883596bd69b15, based on #871b5a4fcf597b9a44fd067e1ed1a3c3cde83aada8.7d01c09passed static checks, production build, 17 mounted notification/Dock/sidebar tests, and 4 isolated opening cases across both engines, including main's 300-author thread. Warm switches were 38.5–40.8ms Chromium / 42–45ms WebKit against the unchanged 100ms limit.622990a: all four Chromium/WebKit journey shards, browser measurements, JavaScript, Rust/tool integration, Windows notifications, security, DCO and CI required. The previously failing WebKit shard now passes with the readiness fix. Final CI run. Native Dock/banner OS acceptance and human desktop visual acceptance remain separate from automated checks.The sidebar improvement was measured using the same production-build fixture: four switches took 16.53/16.49/16.18/15.77ms of scripting before, versus 11.41/11.69/10.87/10.71ms after; parent #87 measured 12.36/10.66/10.51/11.23ms. This diagnostic profiling supports the ownership fix; it does not replace the unchanged acceptance clock or reduce the 128-row/1,001-profile fixture.
Browser coverage
This PR adds two engine cases, one shared-token production CSS journey per engine. Real stylesheet layers, inheritance and feature CSS interactions need a browser. Main's newly inherited presence scenarios remain unchanged. No existing browser journey is removed.
GIF retry/recovery, insertion, panel switching and query preservation remain covered. Avatar decode checks were added to existing light/dark activity cases. Existing thread activity catches the zero-size badge regression. Mounted regressions reproduced AgentChoice and hint failures before their fixes; sidebar tests cover callback replacement and session ownership. No browser budgets, engines, fixture scale, retries or correctness assertions were weakened to obtain passing results.